feat: Allow starting a span on a transaction that has already ended #2653
Conversation
This was a restriction that dated back to the v1 APM Server intake API that required all a transaction's spans to be sent along with the transaction. The Java APM agent, FWIW, allows this.
github-actions
bot
added
the
agent-nodejs
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
|
Here is an example of a trace (in APM UI v7.15.0) that shows a span ("s6") that started after its transaction ended:
This is from running script https://github.com/elastic/apm-agent-nodejs/blob/trentm/otel-bridge/test/opentelemetry-sdk/fixtures/start-span-with-context.js from the ongoing OTel Bridge work. |
This takes the coming changes from #2653 so that tests pass with this restriction lifted.
There was a problem hiding this comment.
Seems reasonable -- especially if java does it. Also, the implementation is reasonable.
Just to say it out loud (not a blocker) I am slightly curious what, if any, effect this will have on features that rely on the transaction's span_count -- it seems like it's possible for these late spans to not be counted (depending on whether an ended transaction's been reported).
A question for future us, I suppose :). Approving, presuming the current CI failures as a :jenkins: hiccup.
Thanks for mentioning it. Yes. We'd discussed this in chat briefly. Felix's opinion was: [Trent]
[Felix]
|
|
/test |
This was a restriction that dated back to the v1 APM Server intake
API that required all a transaction's spans to be sent along with the
transaction.
The Java APM agent, FWIW, allows this.
This came up in discussion about some edge cases with the OTel Bridge. As we move closer to OTel semantics, the distinction between transactions and spans gets to be less. Currently can start a child span of a span that has already ended, so it is nice to be able to do the same for a transaction.
Checklist